Skip to content

Prevent saving during the tutorial - #700

Merged
soloturn merged 2 commits into
MovingBlocks:developfrom
BenjaminAmos:tutorial-save-fixes
Aug 23, 2026
Merged

Prevent saving during the tutorial#700
soloturn merged 2 commits into
MovingBlocks:developfrom
BenjaminAmos:tutorial-save-fixes

Conversation

@BenjaminAmos

@BenjaminAmos BenjaminAmos commented Feb 20, 2024

Copy link
Copy Markdown
Contributor

Description

This pull request attempts to fix areas where the game still saves whilst the tutorial is active. This is only a temporary solution and changes to SaveManager may provide a more comprehensive one in the future.

Testing

  • Start a new game
  • Buy an item
  • Exit to the main menu
  • Start the tutorial
  • Exit the tutorial
  • Continue your last game
  • The item you bought previously, along with your last save data, should be present

This should fix #699.

NicholasBatesNZ
NicholasBatesNZ previously requested changes Aug 14, 2026
View reviewed changes

@NicholasBatesNZ NicholasBatesNZ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, and sorry it sat for so long — it's still needed, #699 is open and I confirmed the autosave timer still overwrites the real save the moment the tutorial starts. I traced every write path on current develop (writeShips, saveWorld, SerialisationManager.serialise) and with this patch they're all covered, so the coverage side looks complete.

One thing to fix before merge though: in StarPort the && !game.isTutorial() guard also skips setTranscendent() (line 127) and setSolShip() (line 405), not just the save — so a tutorial player who uses a star port ends up detached from their ship, since removeObjDelayed(ship) still runs. Star ports are generated in the tutorial world too and the 200 starting credits cover the 10-credit fare, so it's reachable.

Could you keep the isPlayer() check on the outer if and nest only the SaveManager.saveWorld(...) call inside a !game.isTutorial() check? Happy to merge once that's in — no objection to this being a stopgap ahead of a proper SaveManager fix.

BenjaminAmos and others added 2 commits August 23, 2026 15:33
@BenjaminAmos @soloturn
Prevent saving during the tutorial
5167567
@soloturn
fix: only skip the save during tutorial, not the ship/hero linkage
0b1dbe3 
Both StarPort transcendence paths gated setTranscendent()/setSolShip()
behind the same '&& !game.isTutorial()' guard as the save call, so a
tutorial player using a star port got detached from their ship: the
outgoing ship was still removed via removeObjDelayed()/replaced via
addObjDelayed(), but the hero was never pointed at the new object.

Per @NicholasBatesNZ's review: keep the isPlayer() check on the outer
if, nest only the SaveManager.saveWorld() call inside !isTutorial().
@soloturn
soloturn force-pushed the tutorial-save-fixes branch from 147003b to 0b1dbe3 Compare August 23, 2026 13:35
@coderabbitai

coderabbitai Bot commented Aug 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a8a05c6d-0379-47af-948e-c0ad7c8e6562

📥 Commits

Reviewing files that changed from the base of the PR and between 2807a6e and 0b1dbe3.

📒 Files selected for processing (2)
  • engine/src/main/java/org/destinationsol/game/SolGame.java
  • engine/src/main/java/org/destinationsol/game/StarPort.java

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Prevented automatic game and world saves during tutorial gameplay.
    • Preserved saving during regular gameplay, except when the hero is transcendent.

Walkthrough

Tutorial gameplay now skips scheduled world saves and saves triggered by StarPort entry or transcendent travel. Normal gameplay retains the existing save behavior.

Changes

Tutorial Save Protection

Layer / File(s) Summary
Guard tutorial save paths
engine/src/main/java/org/destinationsol/game/SolGame.java, engine/src/main/java/org/destinationsol/game/StarPort.java		 Scheduled saves and StarPort-triggered saves now run only outside tutorial mode. Transcendent heroes remain excluded from scheduled saves.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🟡 Moderate · up to 0b1db

If exiting the tutorial leaves the game marked as being in tutorial mode, normal gameplay saves may remain disabled and later progress could be lost. Verify or fix the tutorial state reset before merging.

Poem

I’m a rabbit with a save to guard,
Tutorial worlds now leave the main one unmarred.
StarPort waits, transcendent travel too,
No tutorial state will overwrite what’s true.
Hop, hop—the save paths now know what to do!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: preventing saves during tutorial gameplay.
Description check ✅ Passed The description explains the tutorial save issue, the intended fix, and the test scenario related to the changes.
Linked Issues check ✅ Passed The changes prevent tutorial saves in the identified ship and StarPort paths, addressing the normal-game save overwrite reported in [#699].
Out of Scope Changes check ✅ Passed The changes are limited to tutorial save prevention in SolGame and StarPort and match the scope of [#699].
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@soloturn

soloturn commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Rebased onto current develop and fixed per your review: both StarPort transcendence paths now keep isPlayer() on the outer if and nest only SaveManager.saveWorld(...) inside !game.isTutorial(), so setTranscendent()/setSolShip() still run during the tutorial and the player stays linked to their ship. Compiles clean.

@soloturn
soloturn dismissed NicholasBatesNZ’s stale review August 23, 2026 13:39

Addressed: StarPort now keeps isPlayer() on the outer if and nests only SaveManager.saveWorld() inside !isTutorial(), so setTranscendent()/setSolShip() still run during the tutorial.

soloturn
soloturn approved these changes Aug 23, 2026
View reviewed changes

@soloturn soloturn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix matches your review exactly: isPlayer() stays on the outer if, only SaveManager.saveWorld() is nested inside !isTutorial() at both call sites. All checks green.

@soloturn
soloturn merged commit 6151ac4 into MovingBlocks:develop Aug 23, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@soloturn soloturn soloturn approved these changes
@NicholasBatesNZ NicholasBatesNZ NicholasBatesNZ left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Starting a tutorial overwrites main game save data

3 participants

@BenjaminAmos @soloturn @NicholasBatesNZ